Skip to content

[11.x] First class callable syntax #53806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed

Conversation

PerryvanderMeer
Copy link
Contributor

This PR removes some redundant anonymous functions and replaces it with PHP's first class callable syntax, improving readability a bit.

@@ -38,7 +38,7 @@ public function handle(PackageManifest $manifest)

(new Collection($manifest->manifest))
->keys()
->each(fn ($description) => $this->components->task($description))
->each($this->components->task(...))
Copy link
Member

@timacdonald timacdonald Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it could be a breaking change?

$this->components->task method will receive the $description as it previously did but it will also receive the each callbacks $index as the second parameter, where it is expecting a callable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed this is a breaking change.

Screenshot 2024-12-10 at 09 19 31

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, totally missed that!

@PerryvanderMeer PerryvanderMeer marked this pull request as draft December 9, 2024 22:47
@crynobone
Copy link
Member

Closing the PR as Laravel 11 is under bug fix and security changes only, you can try submitting the PR to 12.x.

@crynobone crynobone closed this Apr 14, 2025
@PerryvanderMeer PerryvanderMeer deleted the first-class-callables branch April 14, 2025 05:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants